Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(webSocket): create documentation for the operator #2450

Merged
merged 1 commit into from
Jul 26, 2018
Merged

docs(webSocket): create documentation for the operator #2450

merged 1 commit into from
Jul 26, 2018

Conversation

mpodlasin
Copy link
Contributor

Uff.. This one was really dense. ;)

Description

  • Create documentation describing what paramaters webSocket operator accepts and what it returns. * Describe WebSocketSubject API - both how Subject methods behave and what additional multiplex operator does.
  • Add examples for receiving messages, sending messages and multiplexing sockets.
  • Describe properties of WebSocketSubjectConfig object.

Related
I had problems justifying some desing decisions. Answering those might help me create better descriptions:
#2445
#2442

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.688% when pulling c6138e7 on Podlas29:websocket-docs into 69d051b on ReactiveX:master.

@kwonoj kwonoj added the docs Issues and PRs related to documentation label Mar 12, 2017
@ghost
Copy link

ghost commented May 9, 2017

The document updates were really helpful. Thank you!

@huan
Copy link
Contributor

huan commented May 16, 2017

+1

Great document!

@HipsterZipster
Copy link

I see there's a conflict now, but what's the status of this PR? Why hasn't it been merged yet?

@mpodlasin
Copy link
Contributor Author

I'll solve conflicts tommorow and ping maintainers. It wasn't merged because it needs to be reviewed still. :)

@mpodlasin
Copy link
Contributor Author

mpodlasin commented Jul 28, 2017

"tommorow" was 15 days :P Anyways, I resolved conflicts, should be fine now.

Now it's all up to the maintainers @kwonoj @benlesh @staltz

@rxjs-bot
Copy link

rxjs-bot commented Jul 28, 2017

Messages
📖

CJS: 3215.2KB, global: 598.2KB (gzipped: 111.5KB), min: 138.0KB (gzipped: 29.1KB)

Generated by 🚫 dangerJS

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.736% when pulling 66a8bc3 on mpodlasin:websocket-docs into d4edbf6 on ReactiveX:master.

Copy link
Member

@benlesh benlesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost perfect. Just need to subscribe in the pushing messages example. Also be sure to explain that it's queuing messages to be sent until connection occurs.

* @param {string | WebSocketSubjectConfig} urlConfigOrSource the source of the websocket as an url or a structure defining the websocket object
* @return {WebSocketSubject}
* @example <caption>Pushing messages to the server.</caption>
* const subject = Rx.Observable.webSocket('ws://localhost:8081');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example is slightly off, in that it won't push the messages to the server unless you're subscribed. That's when it connects. It will queue the messages and wait for a connection before you subscribe, but at some point you have to subscribe.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added subscription with a comment in the example.

In the text there is already a line which states that sent values are buffered without connection.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a matter of fact, it is still off, since I subscribe and complete immediately after, which I guess means nothing will be sent anyways.

But to make it 100% legit I would have to use openObserver, which would make example super convoluted for newcomers, so for educational reasons I left it as is, hoping comments are clear enough

Create documentation describing what paramaters webSocket
operator accepts and what it returns. Describe WebSocketSubject
API - both how Subject methods behave and what additional multiplex
operator does. Add examples for receiving messages, sending
messages and multiplexing sockets. Describe properties
of WebSocketSubjectConfig object.
@coveralls
Copy link

coveralls commented Jul 28, 2017

Coverage Status

Coverage remained the same at 97.736% when pulling 47ae857 on mpodlasin:websocket-docs into d4edbf6 on ReactiveX:master.

@frankwallis
Copy link

Really glad I found this documentation!

@benlesh
Copy link
Member

benlesh commented Mar 30, 2018

I would love it if we could get these docs updated at merged. The files have moved though, probably better to start a new PR and copy things over.

@mpodlasin can you do that? This time I'll merge them quickly.

@benlesh benlesh merged commit 47ae857 into ReactiveX:master Jul 26, 2018
@benlesh
Copy link
Member

benlesh commented Jul 26, 2018

Thanks again, @mpodlasin ... finally merged.

@huan
Copy link
Contributor

huan commented Jul 27, 2018

Awesome!

@lock lock bot locked as resolved and limited conversation to collaborators Aug 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
docs Issues and PRs related to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants